Fix response when friendship exists #55
Conversation
- Instead of an empty json 200 OK return, 409 CONFLICT - Replace flask.abort with flask_smorest.abort
| friendship.status = "active" | ||
| else: | ||
| return "{}", http_client.OK | ||
| abort(http_client.CONFLICT, message="You are already friends with this player!") |
There was a problem hiding this comment.
This is an API change and we'd need to see how old clients deal with this situation
There was a problem hiding this comment.
Let's assume I want to do just that, i.e. change an API;
- I need a list of all the tenants, presumably from the live tier config
- I find the repos for the clients and read the code; lets assume some of them look fine and should be ok with the change, but some would need minor updates
- In either case, I'd want to test a client against a dev tier (or local) backend to see if all is good with the API change (with or without client modifications).
- That means I'd need the full build environment for all tenants for local testing and to know how to direct them to the correct testing tier (is driftUrl=XXX in .ini the standard way and works everywhere)?
- I'd also need to know the deployment pipeline to publish updates to live for all the other tenants.
Sounds like a lot of time consuming busywork.
There was a problem hiding this comment.
Since you can't realistically check all clients, you might not even have access, the only proper way is to not break APIs, but to version, one way or another.
We know we want versioning, and I would prefer not having to operate multiple versions of every deployable forsaking multi-tenancy, as that suddenly requires versioning the deployment infrastructure instead, and you're basically just moving complexity from the API management to infrastructure management and ops.
So, what we should do, is to figure out API versioning. And until then, we need to simply avoid breaking APIs.
Currently, when creating a friendship that already exists, we return an empty JSON response with status code 200 OK.
I don't think this is restful and returning an empty JSON instead of the "created" resource is unintuitive.
I believe the proper restful way of doing this is 409 CONFLICT since POST is not an idempotent verb.
Also I swapped out
flask.abortforflask_smorest.abortin the friendships API.The responses are essentially the same, but IIRC we want to use that going forward.